Skip to content

feat: add livestream REST endpoints - #32

Open
mrdarrengriffin wants to merge 34 commits into
mainfrom
youtube-api
Open

feat: add livestream REST endpoints#32
mrdarrengriffin wants to merge 34 commits into
mainfrom
youtube-api

Conversation

@mrdarrengriffin

Copy link
Copy Markdown
Member

Adds a livestream endpoint that reports the YouTube livestream status for
Open Home Foundation project channels (Home Assistant, ESPHome, Open Home
Foundation, Music Assistant). Websites can call this to show upcoming, live, or
recently-ended streams.

Endpoints

  • GET /livestream — status for all configured channels
  • GET /livestream/:slug — status for a single channel (e.g. home-assistant);
    returns 404 for unknown slugs

Each entry returns a status of live, upcoming, past (within 24h of
ending), or none, plus title, url, and (for upcoming) startTime.

Notes

  • Channels are config-driven in src/livestream/livestream.channels.ts
    adding a project is a one-line change.
  • Results are cached per channel (5 min TTL) with in-flight de-duplication;
    a failing channel falls back to stale data and doesn't affect the others.
  • Requires a YOUTUBE_API_KEY (YouTube Data API v3), loaded from .env via
    @nestjs/config. See .env.example.
  • Devcontainer now forwards port 3000 (the app's default) instead of 443.

Copilot AI review requested due to automatic review settings July 9, 2026 12:05
@mrdarrengriffin
mrdarrengriffin marked this pull request as draft July 9, 2026 12:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a new NestJS livestream feature area that exposes REST endpoints for querying YouTube livestream state for a configured set of Open Home Foundation project channels, backed by YouTube Data API v3 and cached per channel.

Changes:

  • Add GET /livestream and GET /livestream/:slug endpoints via a new controller/module/service.
  • Implement YouTube API integration with per-channel caching and in-flight refresh de-duplication.
  • Wire up global config loading (@nestjs/config) and add developer/supporting files (.env.example, devcontainer port forwarding, .env gitignore).

Reviewed changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/livestream/livestream.service.ts Implements YouTube API lookups, caching, and status derivation logic for channels.
src/livestream/livestream.controller.ts Adds REST endpoints for listing all channel statuses and querying by slug.
src/livestream/livestream.module.ts Registers the livestream controller/service in a dedicated Nest module.
src/livestream/livestream.channels.ts Defines the config-driven list of tracked channels (slug/name/handle).
src/livestream/index.ts Exposes livestream module/service/channels via barrel exports.
src/app.module.ts Enables global config loading and registers LivestreamModule.
package.json Adds @nestjs/config dependency.
pnpm-lock.yaml Locks @nestjs/config and its transitive dependencies (dotenv, dotenv-expand, etc.).
.gitignore Ensures local .env files are not committed.
.env.example Documents the required YOUTUBE_API_KEY environment variable.
.devcontainer.json Updates forwarded port to 3000 to match the app’s default HTTP port.
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/livestream/livestream.service.ts Outdated
Comment thread src/livestream/livestream.service.ts Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@djwmarcx djwmarcx changed the title Add livestream REST endpoints feat: add livestream REST endpoints Jul 9, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@mrdarrengriffin
mrdarrengriffin marked this pull request as ready for review July 9, 2026 12:13
Copilot AI review requested due to automatic review settings July 9, 2026 12:13
@mrdarrengriffin
mrdarrengriffin requested a review from djwmarcx July 9, 2026 12:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 11 changed files in this pull request and generated 3 comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

Comment thread src/livestream/livestream.service.ts Outdated
Comment thread src/livestream/livestream.service.ts Outdated
Comment thread src/livestream/livestream.service.ts Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 9, 2026 12:20
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 11 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

@djwmarcx djwmarcx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution requires a lot of quota. More than we have available (35x).
We need to come up with a different approach.

What about polling (for each channel):

https://www.youtube.com/feeds/videos.xml?channel_id=UCbX3YkedQunLt7EQAdVxh7w

Plus 1 call to extract the necessary data using the actual API (if really needed)

Copilot AI review requested due to automatic review settings July 9, 2026 14:06
@mrdarrengriffin

Copy link
Copy Markdown
Member Author

@djwmarcx

Switched from search polling to push + adaptive polling

Reworked the livestream feature to cut YouTube API quota while keeping updates near-instant.

Why: search.list costs 100 units/call — polling four channels burns the 10k/day budget fast.

Now:

  • WebSub push for instant "new/updated video" signals (0 quota).
  • videos.list (1 unit) to classify live details.
  • RSS discovery every 5 min (free) to catch scheduled streams push doesn't signal.
  • Adaptive polling: videos.list every 10s while live/imminent, ~60s otherwise, 0 when idle — catches go-live/ended in ~10s.

Reads are served from memory, so endpoints cost no quota per request.

API:

  • GET /livestream — all channels
  • GET /livestream/:slug — one channel

Returns status (live/upcoming/past/none) + title, url, startTime. Channels are config-driven (one-line to add one).

Quota: ~1,080 units for a 3h live event + ~1,150/day discovery — well under 10k.

Deploy: needs YOUTUBE_API_KEY; set PUBLIC_BASE_URL to enable push and PUBSUB_SECRET to verify payloads (see .env.example).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 14 changed files in this pull request and generated 4 comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

Comment thread src/livestream/livestream.service.ts
Comment thread src/livestream/pubsub.service.ts Outdated
Comment thread src/livestream/pubsub.controller.ts Outdated
Comment thread src/livestream/livestream.service.ts Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 9, 2026 14:14
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 14 changed files in this pull request and generated 4 comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

Comment thread src/livestream/pubsub.controller.ts Outdated
Comment thread src/livestream/pubsub.service.ts Outdated
Comment thread src/livestream/livestream.service.ts
Comment thread src/livestream/pubsub.controller.ts Outdated
The project had no test infrastructure. Add jest with ts-jest, @nestjs/testing
and supertest, covering the livestream feature: the service state machine
(status transitions, adaptive poll cadence, quota batching, feed and API
failures), the config parser's validation matrix, the controller, and the HTTP
surface end to end.

The e2e suite also guards the RSS-only decision, so it cannot silently revert:
/pubsub must stay a 404 and nothing may call pubsubhubbub.appspot.com.

tsconfig.build.json keeps spec files out of dist, which nest build would
otherwise compile and ship in the container image. pnpm-workspace.yaml
acknowledges unrs-resolver's build script, a transitive jest-resolve
dependency; its prebuilt binary arrives as an optional dependency, so the
script is skipped as @nestjs/core's is. CI runs both suites on Node 24 with the
release-age cooldown disabled for the frozen install, for the same reason the
container build disables it.
Copilot AI review requested due to automatic review settings July 28, 2026 12:43
@djwmarcx

Copy link
Copy Markdown
Member

Pushed a set of changes to this branch (bbea247, ffbaa41) plus a merge of main to clear the lockfile conflict. PR description intentionally left as-is — happy to discuss any of this before it goes in.

Dropped the WebSub/PubSubHubbub push path — RSS only

  • YouTube's hub notifies on uploads and title/description edits only (docs) — not on a broadcast going live or ending. So push never delivered the signal this feature is about; the reconcile poller did, and still does.
  • Push therefore only bought learning about a newly scheduled stream seconds rather than minutes earlier — for streams typically announced days ahead — in exchange for an unauthenticated public webhook, HMAC verification, subscription leases and renewal timers.
  • Removing it also removes a verification flaw: /pubsub confirmed any hub verification request, so anyone knowing the callback URL could ask the hub to unsubscribe us (killing push until the next renewal), or subscribe us to an arbitrary feed. Per WebSub §5.3 a subscriber must confirm the topic matches a pending request of its own and otherwise return 404.
  • Verified against live YouTube that RSS-only discovery finds a scheduled stream 8 days out — the 2026.8 release party, with the correct startTime.
  • ~260 lines and 3 of 4 env vars gone.

Channels moved to config

  • LIVESTREAM_CHANNELS=home_assistant:home-assistant,esphomeio:esphome,... — comma-separated handle:slug pairs, validated at startup so a bad deploy fails loudly instead of tracking nothing.
  • The slug is pinned in config, not derived from the channel's YouTube name, so renaming a channel on YouTube cannot silently change our public URLs.
  • Display names are no longer configured — read from each feed's channel <title>. Costs no quota (we already fetch the feed) and follows a rebrand without a deploy.
  • Validation catches pasted URLs, non-URL-safe slugs, duplicate slugs, and two entries resolving to the same handle (case-insensitively).

Bug fixes

  • Deleted/privated videos were never untracked. videos.list silently omits IDs it will not serve, and only returned items were iterated — so a stream stuck at live pinned its channel's status and held the 10s poll cadence open indefinitely, which alone could exhaust the daily quota. Eviction happens only on a successful response, so an API failure is never read as "the video is gone".
  • Deleted upcoming streams lingered too — a deleted video also leaves the feed, so it is never queried again. Now dropped when the feed stops listing it (deliberately not applied to live/past, which the poller and the 24h window own).
  • A 200 response that is not an Atom feed would read as "this channel has no videos" and evict tracked streams, and its <title> would be served as the channel name. Now treated as a failed fetch.
  • XML entities in feed titles were served raw — a channel named Nabu Casa's would appear as Nabu Casa&#39;s.
  • updatedAt churned — stamped on every recompute, so it advanced every 10s for the whole duration of a live stream, making it useless for caching or change detection. Now marks when the served state actually changed (verified stable across ticks against live data).
  • startTime was dropped when a stream went live, so a client watching the transition saw it appear and then vanish.

Quota and dead code

  • Discovery now fingerprints each feed (videoId@updated pairs) and skips the videos.list classification when nothing changed, so steady-state discovery is free. YouTube bumps an entry's <updated> on metadata edits, so retitled/rescheduled streams are still caught. This makes shortening the 5-minute interval cheap if we want faster detection.
  • Removed the IDLE_TICKS gate and hasActiveStream(): their predicate was identical to reconcile()'s own candidate filter, so the gate guarded a path that was already a no-op when idle. The comment claiming it saved quota was wrong — idle ticks always cost zero. Behaviour-neutral, and the pre-existing cadence tests passing untouched is the proof.

Tests (there were none)

  • Added jest + ts-jest + @nestjs/testing + supertest: 139 tests (116 unit, 23 e2e), livestream module at ~92% statements.
  • Covers the state machine (transitions, adaptive cadence, quota batching, feed/API failures), the config validation matrix, the controller, and HTTP end to end.
  • e2e guards the RSS-only decision: /pubsub must stay 404 and nothing may call pubsubhubbub.appspot.com.
  • tsconfig.build.json added because nest build would otherwise compile specs into dist and ship them in the image.
  • CI now runs both suites on Node 24.

Not done here (deliberately)

  • helmet/security headers, X-Powered-By, and rate limiting — app-wide middleware, better as their own PR.
  • CORS is not enabled at all, so browsers will block cross-origin calls from our sites — this needs deciding before the endpoint is actually usable, with an explicit origin allowlist rather than a wildcard.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 19 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

package.json:22

  • collectCoverageFrom is using a regex-like glob (src/**/*.(t|j)s) that won’t match *.ts/*.js files under Jest’s globbing, so coverage collection may silently miss source files (or collect nothing). Use a valid glob pattern (and consider excluding *.spec.ts since tests live under src/).
    "collectCoverageFrom": ["src/**/*.(t|j)s"],

src/livestream/livestream.service.ts:100

  • decodeXmlText can throw a RangeError if it encounters a numeric character reference in the UTF-16 surrogate range (0xD800–0xDFFF), because String.fromCodePoint rejects surrogate code points. Since feed titles are untrusted network input, this could break discovery for a channel. Treat surrogate code points as invalid and leave the entity escaped (fall back to match).
    return Number.isInteger(code) && code >= 0 && code <= 0x10ffff
      ? String.fromCodePoint(code)
      : match;

Restructure the repo around the conventions in
https://standards.openhomefoundation.org/ so it looks and behaves like the
foundation's other projects.

Tasks and scripts:
- Add script/ (setup, update, server, test, cibuild) following
  scripts-to-rule-them-all, so contributors and CI run the same code paths.
- Add .mise.toml registering the standard lifecycle tasks (setup, update, dev,
  test, ci) that delegate to script/, plus convenience tasks for lint, format,
  typecheck, build and a REPL.
- Provision the toolchain in the devcontainer via mise (script/install-mise)
  instead of duplicating setup steps in .devcontainer.json.
- Have CI and the release workflow call script/cibuild rather than restating
  the pipeline.

Code style and hygiene:
- Add Prettier and a flat ESLint config, format the tree, and enforce both in
  a husky pre-commit hook via lint-staged.
- Pin the toolchain: packageManager, Node 24 in mise/CI/Dockerfile.
- Harden installs in pnpm-workspace.yaml: minimumReleaseAge of 7 days against
  freshly published malicious releases, and explicit allowBuilds decisions for
  the three dependencies with postinstall scripts.
- Rename .env.example to example.env and document configuration in README.md.
- Ignore the project-local .pnpm-store the devcontainer creates.

API documentation:
- Serve an OpenAPI document and Swagger UI at /docs (JSON at /docs-json),
  versioned from the running build, and annotate the livestream and health
  responses so the spec is generated from the code.
- Cover the served spec with test/swagger.e2e-spec.ts.
Copilot AI review requested due to automatic review settings July 28, 2026 20:30
@djwmarcx

Copy link
Copy Markdown
Member

Pushed 81b9678 — restructures the repo around https://standards.openhomefoundation.org/ so this lands looking like the foundation's other projects. No behaviour change to the livestream endpoints; the one functional addition is the OpenAPI document.

Tasks and scripts

  • script/ (setup, update, server, test, cibuild) following scripts-to-rule-them-all, so contributors and CI run the same code paths.
  • .mise.toml registers the standard lifecycle tasks (setup, update, dev, test, ci), each delegating to script/, plus convenience tasks for lint/format/typecheck/build and a REPL.
  • The devcontainer provisions the toolchain through mise (script/install-mise) instead of restating setup steps in .devcontainer.json.
  • CI and the release workflow call script/cibuild rather than duplicating the pipeline.

Style and hygiene

  • Prettier plus a flat ESLint config, tree formatted, both enforced in a husky pre-commit hook via lint-staged.
  • Toolchain pinned: packageManager, and Node 24 consistently across mise, CI, and the Dockerfile.
  • Install hardening in pnpm-workspace.yaml: a 7-day minimumReleaseAge so a freshly published malicious release can't be picked up immediately, and explicit allowBuilds decisions for the three dependencies with postinstall scripts — all skipped (a funding banner, a third-party analytics beacon, and a native link whose prebuilt binary arrives as an optional dependency).
  • .env.exampleexample.env per the naming standard, and a README.md covering configuration and the task list. Heads-up that the PR description above still points at .env.example.

API documentation

  • OpenAPI document and Swagger UI at /docs (raw JSON at /docs-json), versioned from the running build so the spec a deployment serves always describes that deployment.
  • Livestream and health responses annotated, so the spec is generated from the code instead of maintained by hand.
  • test/swagger.e2e-spec.ts covers the served document.

script/cibuild is green locally: format check, lint (0 errors, 23 pre-existing no-explicit-any warnings), typecheck, 116 unit + 39 e2e tests, build. Happy to type those anys away in a follow-up if we want lint at zero.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 45 out of 48 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

.vscode/settings.json:5

  • This VS Code setting hard-codes a developer-specific absolute path ("/Users/..."), which will not exist for other contributors or in the devcontainer and can break debugging configuration for the team. This file should not contain machine-local paths; either remove the setting or replace it with a portable default.
{
   "debug.javascript.defaultRuntimeExecutable": {
      "pwa-node": "/Users/marcx/.local/share/mise/shims/node"
   }
}

.husky/pre-commit:1

  • This Husky hook is missing a shebang. Git executes hooks as standalone executables; without a shebang the hook can fail with an "exec format error" on Unix-like systems, causing lint-staged to never run.
pnpm exec lint-staged

test/swagger.e2e-spec.ts:37

  • The fetch stub falls back to returning a valid Atom feed for any non-Google URL. That can hide unexpected outbound requests during app boot (wrong host/path) and make this e2e test pass when it shouldn't. Tighten the stub to only handle the exact YouTube API + feed URLs and throw on anything else.

Clears the 23 no-explicit-any warnings, so lint is silent, and restores the
rule to the recommended 'error' now that nothing needs the exemption.

- Model the YouTube Data API surface we actually read as YouTubeItem and
  YouTubeListResponse, and use them for track(), videoDetails() and apiGet().
  Only the fields we request a `part` for are declared, all optional bar `id`,
  and apiGet asserts the parsed body rather than pretending it is validated.
- Reading a real type surfaced two spots the compiler could not check before:
  a channelId lookup that could be passed undefined, and a past-window check
  that reached for actualEndTime through a separate status variable. Both now
  narrow explicitly; behaviour is unchanged in each case.
- Type the health module's async params from Nest's own FactoryProvider rather
  than restating its variadic factory signature, so the two cannot disagree,
  and report health payloads as Record<string, unknown>.
- In the e2e specs, type the supertest target as node:http Server and the
  response entries as LivestreamInfo, which makes each assertion state the
  contract it is checking.
Copilot AI review requested due to automatic review settings July 28, 2026 20:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 45 out of 48 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

.husky/pre-commit:1

  • This Git hook file lacks a shebang, so Git will attempt to execute it directly and it can fail with an exec-format error on POSIX systems. Add a shell shebang (and optionally set -e) and ensure the file is committed with the executable bit set.
pnpm exec lint-staged

.vscode/settings.json:5

  • This setting hard-codes a developer-local absolute path (/Users/...) into the repo, which will break debugging for other contributors and in devcontainers. This should be removed or replaced with a portable configuration.
{
   "debug.javascript.defaultRuntimeExecutable": {
      "pwa-node": "/Users/marcx/.local/share/mise/shims/node"
   }
}

Comment on lines +531 to +534
this.state.set(slug, {
...next,
updatedAt: changed ? new Date().toISOString() : previous.updatedAt,
});
Adds helmet as setupSecurity() in main.ts, applied before Swagger mounts its
routes: Express walks middleware and handlers in registration order, so a route
registered first would answer without the headers.

Helmet's defaults are taken unchanged. The one thing that usually forces a
custom policy is the Swagger UI, and it does not here: every script that page
loads is external and same-origin, which `script-src 'self'` allows, and its
inline <style> blocks are covered by a default `style-src` that includes
'unsafe-inline'.

test/security.e2e-spec.ts pins both halves of that. It asserts the headers on a
JSON collection, a health probe, the OpenAPI document, the UI and a 404, and it
checks the served UI against the policy protecting it — no inline script, no
inline event handler, no off-origin script source. A Swagger release that starts
inlining its init script therefore fails a test rather than silently breaking
the page in browsers.

Verified the suite fails without the middleware before keeping it.
Copilot AI review requested due to automatic review settings July 28, 2026 20:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 46 out of 49 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

.vscode/settings.json:5

  • This appears to be a developer-local VS Code debug setting with an absolute path into a specific user’s home directory. Committing this will break debugging for other contributors and for devcontainers/CI.
{
   "debug.javascript.defaultRuntimeExecutable": {
      "pwa-node": "/Users/marcx/.local/share/mise/shims/node"
   }
}

README.md:28

  • The README claims the OpenAPI YAML document is served at /docs-yaml, but the current Swagger setup only configures the JSON document URL (and tests only cover /docs-json). Either add YAML support in setupSwagger() or update this documentation to avoid advertising a non-existent endpoint.
Interactive documentation is generated from the code and served at
[`/docs`](http://localhost:3000/docs), with the OpenAPI document itself at
`/docs-json` (and `/docs-yaml`).

src/main.ts:55

  • SwaggerModule.setup() is given a custom jsonDocumentUrl, but no yamlDocumentUrl. In @nestjs/swagger, providing custom document URLs typically replaces the defaults, so /docs-yaml may not be served even though it’s referenced elsewhere (README). Consider explicitly configuring the YAML endpoint too (or removing YAML mentions).

The sites that consume this API are deployed independently of it, so who may
call it belongs in configuration: CORS_ORIGINS takes a comma-separated list of
origins, or "*" for any.

Entries are normalised to what a browser actually sends in Origin — scheme,
host, non-default port, lowercased — so a trailing slash or odd casing in the
environment still matches. Anything that could never match is rejected at
startup instead: a bare hostname, a non-http scheme, a path, query, fragment or
credentials, an embedded wildcard, "*" mixed with named origins, and the same
origin listed twice. That follows LIVESTREAM_CHANNELS' precedent — a typo here
would otherwise surface only as a CORS error in someone else's browser console.

Unset means no cross-origin headers at all, with a warning logged at startup.
The API still answers every request; a browser simply will not hand the body to
a page on another site. A deployment that has not said who may read it should
not have that guessed at, and "*" is one character away for anyone who wants it.

Only GET is advertised, since every endpoint here is a read.

Covered by unit specs over the parser and an e2e suite over the served headers:
an allowed origin, a second allowed origin, an unconfigured one, a preflight,
"*", and nothing configured. A third suite needing the same boot made the
harness worth extracting, so swagger and security now share
test/test-app.fixture.ts, which assembles the app exactly as bootstrap() does.

Also verified against a real `node dist/main`: the header is echoed for the
configured origin, Vary: Origin is set, and other origins get nothing.
Copilot AI review requested due to automatic review settings July 28, 2026 20:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 50 out of 53 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

README.md:28

  • README claims the OpenAPI document is served at /docs-json and /docs-yaml, but Swagger is only configured with jsonDocumentUrl in src/main.ts (no YAML document URL is set). This makes the docs inaccurate for consumers.
Interactive documentation is generated from the code and served at
[`/docs`](http://localhost:3000/docs), with the OpenAPI document itself at
`/docs-json` (and `/docs-yaml`).

.vscode/settings.json:5

  • This workspace setting hard-codes a developer-specific absolute path (/Users/...) for the Node runtime. Committing this will break debugging for other contributors and CI/devcontainers. This kind of setting should be user-local, not in the repository.
{
   "debug.javascript.defaultRuntimeExecutable": {
      "pwa-node": "/Users/marcx/.local/share/mise/shims/node"
   }
}

src/livestream/livestream.channels.ts:37

  • The PR description says channels are configured by editing src/livestream/livestream.channels.ts ("adding a project is a one-line change"), but the implementation reads channels from the LIVESTREAM_CHANNELS environment variable via parseChannels(). The PR description should be updated to match the env-driven configuration model.

It pinned debug.javascript.defaultRuntimeExecutable to a path under one
developer's home directory, which does not exist for other contributors or in
the devcontainer. Shared editor configuration belongs in .devcontainer.json,
which already sets the formatter and the required extensions.
Copilot AI review requested due to automatic review settings July 28, 2026 21:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 49 out of 52 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/main.ts:56

  • The warning message here is triggered whenever parseCorsOrigins() returns an empty list, which includes both an unset and a blank CORS_ORIGINS value. The current text says "is not set", which can mislead operators who set the variable but left it empty/whitespace.

The foundation's sites are spread across subdomains — www, developers, data —
and enumerating them in CORS_ORIGINS would mean a deploy every time one appears.
An entry may now name a domain's subdomains instead: https://*.esphome.io.

Matching is an anchored pattern per entry, so it stays narrow. It covers any
depth of subdomain (www.esphome.io, a.b.esphome.io) but not the bare domain —
list that separately when it also serves pages — and a literal dot before the
domain keeps a lookalike like evil-esphome.io out, as the anchor keeps out
esphome.io.evil.example. Scheme and port must still match exactly.

A wildcard anywhere other than the leading label is rejected, as is one covering
a whole suffix ("*.io"), since that would hand every .io site access. "*" alone
still means any origin, and still cannot be mixed with named entries.
Copilot AI review requested due to automatic review settings July 28, 2026 21:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 49 out of 52 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/main.ts:105

  • README.md advertises a /docs-yaml OpenAPI endpoint, but setupSwagger() only configures the JSON document URL. In current @nestjs/swagger, the YAML endpoint is only exposed when yamlDocumentUrl is set, so /docs-yaml will likely 404 despite being documented. Either add yamlDocumentUrl (recommended) or remove the YAML mention from the README.
    README.md:66
  • PR description says channels are “config-driven in src/livestream/livestream.channels.ts (adding a project is a one-line change)”, but the implementation/config docs here indicate channels are driven by the LIVESTREAM_CHANNELS environment variable (with defaults in example.env). Please update the PR description to match the actual configuration mechanism so reviewers/operators aren’t misled.
Configuration is environment variables only. `example.env` documents every one;
copy it to `.env` for local development (`.env` is gitignored and must never be
committed). In production these are set on the container.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants